Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Rich Text Editor | Add label provider in nimble-rich-text-editor #1459

Merged
merged 20 commits into from
Sep 6, 2023
Merged

Rich Text Editor | Add label provider in nimble-rich-text-editor #1459

merged 20 commits into from
Sep 6, 2023

Conversation

aagash-ni
Copy link
Contributor

@aagash-ni aagash-ni commented Aug 28, 2023 •
edited
Loading

Pull Request

🤨 Rationale

Issue Link : #1288

Currently, rich-text-editor has title and text content for formatting buttons (Bold, Italics, Bulleted List, Numbered List) and this needs to be localized from the client side.

👩‍💻 Implementation

Reference PR for implementation: #1328

This PR creates label DesignTokens in nimble-rich-text-editor

Note: Angular Integration will be taken up once after this PR completion.

🧪 Testing

  • Verified that each label appears in the expected spots in the DOM for the affected components. Note that the current labels are mostly not user-visible. Future format button additions will introduce new visible label tokens.
  • Added autotests
    • For label providers: verify token naming, and that label provider attribute updates result in token updates
  • Updated Storybook
    • Rich Text Editor has a new "Localizable labels" row in their Storybook controls/API section
    • Under Tokens/Label Providers is a new Rich Text Editor Label Provider.

✅ Checklist

  • I have updated the project documentation to reflect my changes or determined no changes are needed.

@aagash-ni aagash-ni requested a review from vikisekarNI August 29, 2023 10:29
@aagash-ni aagash-ni requested a review from vikisekarNI August 29, 2023 11:29
@vikisekarNI vikisekarNI requested a review from m-akinc August 29, 2023 12:09
@aagash-ni aagash-ni marked this pull request as ready for review August 30, 2023 18:15
@jattasNI jattasNI requested a review from msmithNI August 30, 2023 20:54
jattasNI
jattasNI approved these changes Aug 30, 2023
View reviewed changes
Copy link
Contributor

@jattasNI jattasNI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added @msmithNI as an optional reviewer since he created the label provider infrastructure. But you don't need to wait on his approval. Now that Milan's back in office, you should wait for his review though.

packages/nimble-components/src/label-provider/rich-text-editor/label-token-defaults.ts Outdated Show resolved Hide resolved
packages/nimble-components/src/label-provider/rich-text-editor/name-utils.ts Outdated Show resolved Hide resolved
packages/nimble-components/src/rich-text-editor/tests/rich-text-editor-labels.spec.ts Outdated Show resolved Hide resolved
rajsite
rajsite reviewed Aug 30, 2023
View reviewed changes
packages/nimble-components/CONTRIBUTING.md Outdated Show resolved Hide resolved
packages/nimble-components/src/label-provider/base/tests/label-providers.mdx Outdated Show resolved Hide resolved
packages/nimble-components/src/label-provider/rich-text-editor/label-token-defaults.ts Outdated Show resolved Hide resolved
packages/nimble-components/CONTRIBUTING.md Outdated Show resolved Hide resolved
packages/nimble-components/src/label-provider/rich-text-editor/label-tokens.ts Outdated Show resolved Hide resolved
packages/nimble-components/src/label-provider/rich-text-editor/name-utils.ts Outdated Show resolved Hide resolved
packages/nimble-components/src/rich-text-editor/tests/rich-text-editor-labels.spec.ts Outdated Show resolved Hide resolved
@aagash-ni aagash-ni requested a review from rajsite August 31, 2023 18:32
@aagash-ni aagash-ni requested a review from rajsite September 4, 2023 13:37
@rajsite rajsite enabled auto-merge (squash) September 5, 2023 23:01
@rajsite rajsite merged commit 75dcf39 into main Sep 6, 2023
@rajsite rajsite deleted the users/aagash/editor-globalization branch September 6, 2023 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vikisekarNI vikisekarNI vikisekarNI approved these changes

@msmithNI msmithNI msmithNI approved these changes

@jattasNI jattasNI jattasNI approved these changes

@m-akinc m-akinc m-akinc approved these changes

@rajsite rajsite rajsite approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@aagash-ni @rajsite @m-akinc @jattasNI @msmithNI @vikisekarNI @AagashRaaj